Strip all package types from inner multi-rid tool packages so they don't appear on search-based experiences #49616
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n't appear on search-based experiences
joelverhagen
reviewed
Jul 1, 2025
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets
Outdated
Show resolved
Hide resolved
joelverhagen
reviewed
Jul 1, 2025
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that inner RID-specific tool packages only carry the DotnetToolRidPackage type while the outer tool package retains all specified package types, preventing inner packages from appearing in search-based experiences.
- Extended
TestToolBuilderto accept and embed additional package types into its package identifier. - Added an end-to-end test
StripsPackageTypesFromInnerToolPackagesto validate package types on both inner and outer tool packages. - Updated
Microsoft.NET.PackTool.targetsto conditionally setPackageTypeonly for outer (DotnetTool) vs. inner (DotnetToolRidPackage) packages.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/Microsoft.DotNet.PackageInstall.Tests/TestToolBuilder.cs | Added AdditionalPackageTypes setting and enhanced GetIdentifier |
| test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs | New E2E test covering package-type stripping and related helpers |
| src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets | Conditional logic for outer vs. inner tool package type assignment |
Comments suppressed due to low confidence (3)
test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs:358
- The assertion message only mentions 'DotnetTool' while the test expects
DotnetToolplus additional types; consider updating the message to reference all expected package types or generalize it.
.And.BeEquivalentTo(expectedPackageTypes, "The PackageType should be 'DotnetTool'.");
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets:388
- MSBuild
Conditionexpressions require==for equality checks; using a single=will not evaluate as intended. Update toCondition="$(_ToolPackageType) == 'DotnetTool'".
<AddPackageType Condition="$(_ToolPackageType) = 'DotnetTool'" CurrentPackageType="$(PackageType)" PackageTypeToAdd="$(_ToolPackageType)">
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets:392
- MSBuild
Conditionexpressions require==for equality; update toCondition="$(_ToolPackageType) == 'DotnetToolRidPackage'"so the inner package rule applies correctly.
<PropertyGroup Condition="$(_ToolPackageType) = 'DotnetToolRidPackage'">
Co-authored-by: Joel Verhagen <[email protected]>
dsplaisted
approved these changes
Jul 2, 2025
baronfel
deleted the
strip-packagetypes-from-internal-multi-rid-tool-packages
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #49615
The outer tool package will have all package types (McpServer, DotnetTool), but the inner packages will just have DotnetToolRidPackage. This prevents experiences like the MCP registry, which search based on "has PackageType of McpServer", from reporting the inner packages.